🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Credentials with neither HostName nor MACAddress vanish silently.

If cred.HostName == "" and cred.MACAddress == "", the credential is dropped without error or log — no file is written and nothing signals the misconfiguration to the operator.

 	for _, cred := range fencingCredentials.Config.Credentials {
-		if cred.HostName != "" {
+		switch {
+		case cred.HostName != "":
 			hostnameCredentials = append(hostnameCredentials, cred)
-		} else if cred.MACAddress != "" {
+		case cred.MACAddress != "":
 			macCredentials = append(macCredentials, cred)
+		default:
+			return errors.New("fencing credential must specify either a hostname or a MAC address")
 		}
 	}

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/asset/agent/image/ignition.go` around lines 655 - 661, Update the
credential classification loop in the fencing credentials processing flow to
explicitly handle entries where both HostName and MACAddress are empty. Surface
these malformed credentials through the existing error or logging mechanism
instead of silently dropping them, while preserving the current hostname and
MAC-address collection behavior.

@fracappa the comment seems to be relevant, do you maybe already have somewhere else a validation for that (I assume so)

@andfasano, yes. We already validate it at installation time, before even reaching asset generation.

We don't need to duplicate it in ignition.go, by the time addFencingCredentials runs, the install-config has already been validated.

Just as a reminder, not specifically for this PR: currently this feature is not supported in the twin asset UnconfiguredIgnition{} - the one that is currently used for the OVE ISO, so probably we'll need to have it in the unconfigured ignition as well (cc @zaneb)

This looks a lot like configuration, so for the appliance it needs to be handled in the config-iso (not the unconfigured-ignition) and for the OVE ISO it needs to be configured through the Assisted GUI.

Ok makes sense.

@fracappa the comment seems to be relevant, do you maybe already have somewhere else a validation for that (I assume so)

I think this part is mixing two different things.

1. I would leave the old addFencingCredentials as it is, and simply rename it as addFencingCredentialsByHostname - leaving unchanged the previous responsibility to create the single top-level /etc/assisted/hostconfig/fencing-credentials.yaml. That part worked fine.

2. In the ignition asset, the addHostConfig() method is currently the location taking care of generating specific configuration files per hosts. Currently it simply inherits and processes all the config files from agentHosts.HostConfigFiles(). The AgentHosts was defined as an intermediate asset to abstract the current list of hosts processed (it could be either from agent-config.yaml or nodes-config.yaml).
   I think it could be worth adding a FencingCredentialsHosts field to the AgentHosts struct and manage it in the HostConfigFiles method, separately after the main loop, ie:

...
for i, host := range a.Hosts {
...
}

for i, host := range a.FencingCredentialsHosts {
...
}

Clearly the Generate method should take care of populating the FencingCredentialsHosts when it makes sense. I would not touch/extend the Hosts since that field was meant to handle configs from agent-config.yaml

With this approach, I think that the FC config per host will become totally transparent for the Ignition asset, and more smoothly integrated with the current design: it will become part of the AgentHost, with the notable exception that instead of being populated by from agent-config.yaml in such case the source will be the install-config.yaml FC field.

This is not required, see below

As per the above comments, it looks like this field could be simply a FencingCredentialsHosts []*types.Credential

nit: a comment remembering that this method takes care only to store per host FC data only in case the mac-address is configured won't hurt

IIUC an host could get just a single credential, so a list here sounds unnecessary, isn't it?

At this level the only code that matters is to append an entry to a.FencingCredentialsHosts only if it was configured by mac, ie:

for _, cred := range installConfig.Config.ControlPlane.Fencing.Credentials {
        // filter out hosts without a mac-address configured
		if cred.HostName != "" || cred.MACAddress == "" {
			continue
		}
        a.FencingCredentialsHosts = append(a.FencingCredentialsHost, installConfig.Config.ControlPlane.Fencing.Credentials)
}

All the rest is not required.

Note that in general, for this asset, the concept of "dir" is not known. It only takes care of carrying around hosts list, and provide a file definition for them

The goal here is to serialize the FC file in one of the eligible hosts-<N> folder. In particular the one whose mac-address(es) matches the current FC one. So it looks like a good spot to reuse findHostDirForMAC.
Anyhow, since the FencingCredentials are defined in the install-config.yaml, and the hosts in the agent-config.yaml, things can become a little bit tricky, since there's no guarantee that an abi host would exist for a given FC entry. So, I would recommend a slightly different logic for the matching:

• Iterate over files, looking for keys matching <host folder name>/mac_addresses
  • Check if one the macs matches your current FC mac. If so, get <host folder name> as your current location to store the fencing-credentials.yaml for the FC
• In case of no match, a new host folder will have to be generated - just ensure to not reuse a previously defined host folder name